Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add pipeline configuration for cleaning up upstream branches #1088

Merged
4 commits merged into from
Oct 14, 2020

Conversation

chidozieononiwu
Copy link
Member

  • Add Cleanup pipeline.
  • The intention is to run this on a schedule. The first step here is for cleaning up eng sommon sync branches.

@chidozieononiwu
Copy link
Member Author

/azp run azure-sdk-tools - sync - eng-common

@azure-pipelines
Copy link

No pipelines are associated with this pull request.

@chidozieononiwu
Copy link
Member Author

/azp run azure-sdk-tools - sync - eng-common

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@azure-sdk
Copy link
Collaborator

The following pipelines have been queued for testing:
java - template
js - template
net - template
python - template
Sign off on the approval gate to test the release stage of each pipeline.

@azure-sdk
Copy link
Collaborator

The following pipelines have been queued for testing:
java - template
js - template
net - template
python - template
Sign off on the approval gate to test the release stage of each pipeline.

workingDirectory: $(System.DefaultWorkingDirectory)
filePath: $(System.DefaultWorkingDirectory)/eng/common/scripts/Delete-RemoteBranches.ps1
arguments: >
-RepoName ${{ repo }}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks like you are not passing RepoOwner. I assume you plan to pass azure-sdk as these are cleaning up branches in the forks.

We should also consider doing a similar branch clean-up for the increment version branches.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actually , This is for cleaning up upstream branches pushed to the main repos as part of the new process. So RepoOwner is azure. I was defaulting it in the script earlier.

Do we need to do cleanup on the 'azure-sdk' owned repos?

Copy link
Member

@weshaggard weshaggard left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Add a couple comments but looks good overall.

ghost pushed a commit that referenced this pull request Oct 13, 2020
- Refactor GitHub API call to allow reusability.
- Add Clean up pipeline with step that deletes upstream sync branches . Moved to #1088
- Add auto-merge label to Sync and Tools PRs
- Remove the Verify and Merge stage.
@azure-sdk
Copy link
Collaborator

The following pipelines have been queued for testing:
java - template
js - template
net - template
python - template
You can sign off on the approval gate to test the release stage of each pipeline.
See eng/common workflow

Copy link
Contributor

@mitchdenny mitchdenny left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just an open question about whether we need to delete the pull request as well, but other than that looks good.

@azure-sdk
Copy link
Collaborator

The following pipelines have been queued for testing:
java - template
js - template
net - template
python - template
You can sign off on the approval gate to test the release stage of each pipeline.
See eng/common workflow

@chidozieononiwu chidozieononiwu added Central-EngSys This issue is owned by the Engineering System team. EngSys This issue is impacting the engineering system. labels Oct 13, 2020
@chidozieononiwu chidozieononiwu removed Central-EngSys This issue is owned by the Engineering System team. EngSys This issue is impacting the engineering system. labels Oct 13, 2020
@chidozieononiwu chidozieononiwu self-assigned this Oct 13, 2020
ghost pushed a commit to Azure/azure-sdk-for-c that referenced this pull request Oct 14, 2020
ghost pushed a commit to Azure/azure-sdk-for-go that referenced this pull request Oct 14, 2020
ghost pushed a commit to Azure/azure-sdk-for-cpp that referenced this pull request Oct 14, 2020
ghost pushed a commit to Azure/azure-sdk-for-java that referenced this pull request Oct 14, 2020
ghost pushed a commit to Azure/azure-sdk-for-net that referenced this pull request Oct 14, 2020
@ghost
Copy link

ghost commented Oct 14, 2020

Hello @azure-sdk!

Because this pull request has the auto-merge label, I will be glad to assist with helping to merge this pull request once all check-in policies pass.

p.s. you can customize the way I help with merging this pull request, such as holding this pull request until a specific person approves. Simply @mention me (@msftbot) and give me an instruction to get started! Learn more here.

@ghost ghost merged commit b93718e into Azure:master Oct 14, 2020
@weshaggard
Copy link
Member

@chidozieononiwu one side-effect of using the auto-merge tag we end up loosing some of the commit history when we it squashes and merges. See Azure/azure-sdk-for-c@639007a which is the auto-merged commit for Azure/azure-sdk-for-c#1426 and compare that to the manually merged commit Azure/azure-sdk-for-python#14482 for Azure/azure-sdk-for-python#14482.

The auto-merge will use the issue description as the commit message which is by design. So I guess we should decided if that is OK for our workflow or not. On one hand I kind of like it because it has a direct link to the tools PR but on the other we do lose the commit messages from the tools changes which does force people to always click through to the tools PR.

@mitchdenny what is your thoughts on what should happen here?

Ref to search for
Pass 'heads/<branchame> ,tags/<tag name>, or nothing
#>
function List-References {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we should have a better name for this function as List-References without context is hard to understand as it is very ambiguous with other things. For example reference means something in PS as well. Perhaps List-GithubSourceReferences or something like that might be a little clearer.

$RepoOwner,
[Parameter(Mandatory = $true)]
$RepoName,
$Ref
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

At some point we might also need to pass a token here if we ever call this on a private repo for example.

@@ -229,4 +268,27 @@ function Update-Issue {
}

return Invoke-GitHubAPIPatch -apiURI $uri -body $parameters -token $AuthToken
}

function Delete-References {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Perhaps a better name would be Delete-GithubSourceReference.

exit 1
}

if ($pullRequests.Count -eq 0)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we log cases where we find branches that have a pull request and list the branch and what open pull requests there are still for it?

benbp pushed a commit that referenced this pull request Dec 1, 2020
- Refactor GitHub API call to allow reusability.
- Add Clean up pipeline with step that deletes upstream sync branches . Moved to #1088
- Add auto-merge label to Sync and Tools PRs
- Remove the Verify and Merge stage.
benbp pushed a commit that referenced this pull request Dec 1, 2020
- Add Cleanup pipeline.
- The intention is to run this on a schedule. The first step here is for cleaning up `eng sommon sync` branches.
annelo-msft pushed a commit to annelo-msft/azure-sdk-for-net that referenced this pull request Feb 17, 2021
This pull request was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants